Skip to content

Conversation

@NathanFlurry
Copy link
Member

@NathanFlurry NathanFlurry commented Jan 8, 2026

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 8, 2026

🚅 Deployed to the rivet-pr-3794 environment in rivet-frontend

Service Status Web Updated (UTC)
website ✅ Success (View Logs) Web Jan 14, 2026 at 11:40 pm
frontend-cloud-staging ❌ Build Failed (View Logs) Web Jan 14, 2026 at 11:40 pm
frontend-inspector 🕒 Building (View Logs) Web Jan 14, 2026 at 11:40 pm
frontend-cloud 🕒 Building (View Logs) Web Jan 14, 2026 at 11:40 pm

🚅 Environment rivet-pr-3794 in rivet-frontend-website-preview has no services deployed.

Copy link
Member Author

NathanFlurry commented Jan 8, 2026

@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 8, 2026 21:01 Destroyed
@railway-app railway-app bot temporarily deployed to rivet-frontend-website-preview / rivet-pr-3794 January 8, 2026 21:01 Destroyed
@claude
Copy link

claude bot commented Jan 8, 2026

PR Review: Epoxy Debugging Support

This PR adds comprehensive debugging tools for the Epoxy distributed consensus system. Overall, the implementation is solid with good structure and error handling. Here are my findings:

✅ Strengths

  1. Well-structured API endpoints - Clear separation between replica debug, key debug, and KV operations
  2. Good documentation - Functions have clear doc comments explaining their purpose and behavior
  3. Error handling - Proper use of anyhow::Result and .context() for error propagation
  4. Fanout pattern - Nice implementation of gathering debug info across all replicas with error tracking
  5. Code reuse - Uses existing rivet_pools::reqwest::client() as per guidelines
  6. Type safety - Base64 encoding/decoding with proper validation

🔍 Issues Found

1. Security: No Authentication/Authorization (Critical)

The new debug endpoints in api-peer/src/internal.rs and api-peer/src/router.rs have no authentication or authorization checks. These endpoints expose sensitive cluster state and allow writes:

  • GET /epoxy/replica/debug - exposes cluster config and state
  • GET /epoxy/replica/key/{key} - exposes key instance data
  • PUT /epoxy/replica/kv/{key} - allows writing arbitrary values

Recommendation: Add authentication middleware or at minimum restrict these endpoints to internal/peer traffic only. The write endpoint (set_epoxy_kv) is particularly concerning.

2. Error Handling: Missing Timeout on Fanout Requests

In get_epoxy_key_debug_fanout (internal.rs:451), HTTP requests to other replicas have no timeout:

let response_result = client.get(&url).send().await;

If a replica is unresponsive, this could hang indefinitely.

Recommendation: Add a timeout using tokio::time::timeout or configure the reqwest client with a default timeout.

3. Performance: Sequential Fanout Requests

In get_epoxy_key_debug_fanout (internal.rs:441-477), requests to replicas are made sequentially in a loop:

for replica_config in &config.replicas {
    // ... sequential HTTP request
}

Recommendation: Use futures::future::join_all or tokio::spawn to parallelize these requests for better performance.

4. Code Style: Explicit std::result::Result Types

Lines 453, 455, 459 in internal.rs use fully qualified std::result::Result:: which is unusual in Rust code:

std::result::Result::Ok(response) => { ... }
std::result::Result::Err(e) => { ... }

Recommendation: Simply use Ok and Err - the Result type is already in scope via anyhow::Result.

5. Logging: Missing Structured Logging

The CLI commands use println! for output, which is fine for CLI tools. However, the API endpoints don't have any tracing/logging for debugging production issues.

Recommendation: Add structured logging per CLAUDE.md guidelines:

tracing::info!(?replica_id, "fetching epoxy replica debug info");

6. Documentation: Inconsistent Comment Style

Line 218 in internal.rs: "Returns debug information for this epoxy replica..."
Line 331: "Returns debug information for a specific key..."

The comments are good but could be more concise per CLAUDE.md guidelines (avoid verbose structures).

📝 Minor Issues

  1. Dependency ordering in Cargo.toml: The changes alphabetize dependencies which is good, but this could be done in a separate cleanup commit
  2. Type hints duplication: type_hint parameter appears in multiple CLI commands - could be extracted to a shared struct/trait
  3. Base64 encoding: Multiple places decode base64 - could extract to a helper function for consistency

🧪 Testing Concerns

The PR doesn't include any tests. For debugging tools this might be acceptable, but consider adding:

  • Unit tests for encode_key function (tests both base64 and tuple parsing)
  • Integration test for the fanout behavior
  • Error case tests (invalid base64, missing replica, etc.)

✨ Nice Additions

  1. The instances_by_status aggregation in GetEpoxyKeyDebugResponse is a nice UX touch
  2. Clear separation between local and optimistic reads
  3. Comprehensive CLI with multiple subcommands
  4. Good reuse of existing UDB tuple parsing utilities

🎯 Priority Recommendations

Must Fix Before Merge:

  1. Add authentication/authorization to the new API endpoints (especially set_epoxy_kv)
  2. Add timeout to fanout HTTP requests

Should Fix:
3. Parallelize fanout requests for better performance
4. Simplify Result type usage (remove std::result::Result::)
5. Add structured logging to API endpoints

Nice to Have:
6. Add basic tests for key encoding/decoding
7. Extract common patterns (base64 decoding, type hints) to helper functions

Overall this is a solid debugging feature that will be very useful for operating Epoxy clusters. The main concerns are around security and the missing timeout on network requests.

@NathanFlurry NathanFlurry changed the base branch from 01-07-chore_example_update_examples_to_use_srvx to graphite-base/3794 January 8, 2026 21:25
@NathanFlurry NathanFlurry force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from 08f2cb5 to caeb405 Compare January 8, 2026 23:49
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 8, 2026 23:49 Destroyed
@NathanFlurry NathanFlurry changed the base branch from graphite-base/3794 to main January 8, 2026 23:49
@NathanFlurry NathanFlurry marked this pull request as draft January 8, 2026 23:49
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 8, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3794

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3794

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3794

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3794

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3794

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3794

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3794

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3794

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3794

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3794

commit: fbe3ebc

@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from caeb405 to d789061 Compare January 10, 2026 02:00
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 10, 2026 02:00 Destroyed
@vercel
Copy link

vercel bot commented Jan 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
rivet-cloud Ready Ready Preview, Comment Jan 10, 2026 2:01am
rivetkit-serverless Error Error Jan 10, 2026 2:01am
2 Skipped Deployments
Project Deployment Review Updated (UTC)
rivet-inspector Ignored Ignored Preview Jan 10, 2026 2:01am
rivet-site Ignored Ignored Preview Jan 10, 2026 2:01am

@linear
Copy link

linear bot commented Jan 12, 2026

This was referenced Jan 12, 2026
@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from df64d01 to 0a56839 Compare January 13, 2026 00:44
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 13, 2026 00:44 Destroyed
@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from 0a56839 to e11706d Compare January 14, 2026 03:06
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 14, 2026 03:06 Destroyed
@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from e11706d to fbe3ebc Compare January 14, 2026 19:47
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 14, 2026 19:47 Destroyed
@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from fbe3ebc to d3bc9c1 Compare January 14, 2026 23:02
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 14, 2026 23:02 Destroyed
@MasterPtato MasterPtato force-pushed the 01-08-chore_engine_add_support_for_epoxy_debugging branch from d3bc9c1 to 31d3705 Compare January 14, 2026 23:39
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3794 January 14, 2026 23:39 Destroyed
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 14, 2026

Merge activity

  • Jan 14, 11:40 PM UTC: MasterPtato added this pull request to the Graphite merge queue.
  • Jan 14, 11:41 PM UTC: CI is running for this pull request on a draft pull request (#3908) due to your merge queue CI optimization settings.
  • Jan 14, 11:42 PM UTC: Merged by the Graphite merge queue via draft PR: #3908.

@graphite-app graphite-app bot closed this Jan 14, 2026
@graphite-app graphite-app bot deleted the 01-08-chore_engine_add_support_for_epoxy_debugging branch January 14, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants